Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose all arguments in from_* methods #304

Closed
wants to merge 1 commit into from

Conversation

maringuu
Copy link
Contributor

As a user I don't really care about the Magic class. All I want is to query the magic of some files.
Before this commit you had to create an instance of Magic. After this commit you can pass all arguments you would pass to its constructor to the instanceless methods.

I am pretty happy with this API, are you?

As a user I don't really care about the `Magic` class.
All I want is to query the magic of some files.
Before this commit you had to create an instance of Magic.
After this commit you can pass all arguments you would pass
to its constructor to the instanceless methods.
@ahupp
Copy link
Owner

ahupp commented Oct 13, 2023

From the perspective of a new user of the library, the magic.from_* has a very simple signature that's immediately clear how to use. The magic.Magic class takes a large number of arguments (some obscure) and is intended for cases like yours where you might want customization. So the split here is intentional.

Is it that arduous to create a new instance?

m = magic.Magic(<some arguments)
m.from_file(...)

vs

magic.from_file(..., <some arguments>)

Separate from that, **kwargs on python functions is a usability pain because it's not clear what args it takes.

@ahupp ahupp closed this Oct 13, 2023
@maringuu
Copy link
Contributor Author

Is it that arduous to create a new instance?

Well kind of. I just don't want to keep track of the instances. So I'll create an instance for every call I make, which I don't like.
I figured since there already is some logic to keep track of instances this change would be a good fit.

the magic.from_* has a very simple signature that's immediately clear how to use.

I don't see how they are more intuitive in status quo.
You have to read the docstring to know what it does (I agree that it is kind of obvious for the mime argument).
With this change you would read the docstring then head over to Magic and see what you want there.

large number of arguments (some obscure)

I would argue that you should not hide these arguments for the sake of simplicity. They already have defaults so the user can just rely on them. What is the downside of pointing the user to the full docs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants